Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix VoIP event tile issues #6471

Merged
merged 10 commits into from
Jul 27, 2021
Merged

Fix VoIP event tile issues #6471

merged 10 commits into from
Jul 27, 2021

Conversation

SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Jul 25, 2021 •
edited
Loading

Fixes element-hq/element-web#18216

Screenshot_20210725_091701
Screenshot_20210725_091651
Screenshot_20210725_091832
Screenshot_20210725_091821
Screenshot_20210725_102914
Screenshot_20210725_105731

element-web notes: none

@SimonBrandner
Add missed call icons
4aa8d8f 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Correct iconography for missed calls
aedd535 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Add state for when the remote doesn't pick up
4784ddc 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Support for getting reject info
f63b6fc 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Add declined state
f7087d5 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Cleanup hangup dialogs
7c80550 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Tweak copy
a2c7035 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
i18n
456800a 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner
Add comment
8c63588 
Signed-off-by: Šimon Brandner <[email protected]>
@SimonBrandner SimonBrandner force-pushed the fix/voip-event-tiles/18216 branch from be28f4e to 8c63588 Compare July 25, 2021 09:06
@SimonBrandner SimonBrandner marked this pull request as ready for review July 25, 2021 09:13
@germain-gg germain-gg requested review from a team July 26, 2021 07:04
germain-gg
germain-gg approved these changes Jul 26, 2021
View reviewed changes
Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me code wise!
When message bubbles are enabled the tiles are collapsed, but that will be fixed by #6465
Will make sure that a design review is fast tracked for this PR to unblock RC tomorrow

Thank you for your continuous contributions 💯

@germain-gg germain-gg requested a review from janogarcia July 26, 2021 15:56
janogarcia
janogarcia suggested changes Jul 26, 2021
View reviewed changes
Copy link
Contributor

@janogarcia janogarcia left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Šimon!

Just a couple details we'd need to update to get this PR ready for the next RC.

  • Iconography: Update the missed-voice.svg icon to the one I'm attaching (it follows the latest Design System specs)
  • Copy: Update the i18n strings "The remote side declined this call" and "The remote side didn't pick up", by replacing "The remote side" part with just "They".

missed-voice.svg.zip

@SimonBrandner
Copy link
Contributor Author

SimonBrandner commented Jul 26, 2021 •
edited
Loading

Thanks for the review!

Iconography: Update the missed-video.svg icon to the one I'm attaching (it follows the latest Design System specs)

I am a little confused, sorry - should I use the missed voice call icon even for missed video calls?

(the current icon should be from https://www.figma.com/file/V6m2z0oAtUV1l8MdyIrAep/VoIP?node-id=2863%3A45199)

Copy: Update the i18n strings "The remote side declined this call" and "The remote side didn't pick up", by replacing "The remote side" part with just "They".

Yeah, sounds much better

@SimonBrandner
Update copy
29b0d03 
Signed-off-by: Šimon Brandner <[email protected]>
@janogarcia
Copy link
Contributor

Sorry for the confusion, Šimon! It actually was missed-voice.svg, not missed-video.svg. 🙃 Anyway, please ignore my comment on updating it, as there's some deviation there in place from the design system that seems to be intentional.

@SimonBrandner
Copy link
Contributor Author

Sorry for the confusion, Šimon! It actually was missed-voice.svg, not missed-video.svg. upside_down_face Anyway, please ignore my comment on updating it, as there's some deviation there in place from the design system that seems to be intentional.

Ah, no worries 😄 I'll revert the change

@SimonBrandner SimonBrandner force-pushed the fix/voip-event-tiles/18216 branch from c9dfe74 to 29b0d03 Compare July 26, 2021 19:42
@SimonBrandner
Copy link
Contributor Author

SimonBrandner commented Jul 26, 2021 •
edited
Loading

Do you need me to retake the screenshots? (everything should be updated now)

@janogarcia
Copy link
Contributor

Don't bother taking new screenshots. Thanks anyway!

@germain-gg germain-gg enabled auto-merge July 27, 2021 07:16
@germain-gg germain-gg disabled auto-merge July 27, 2021 07:16
@germain-gg germain-gg merged commit 788abac into matrix-org:develop Jul 27, 2021
@SimonBrandner SimonBrandner deleted the fix/voip-event-tiles/18216 branch July 27, 2021 07:18
@gaelledel gaelledel mentioned this pull request Aug 4, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@janogarcia janogarcia janogarcia requested changes

@germain-gg germain-gg germain-gg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

VoIP event tiles UI/UX issues
3 participants
@SimonBrandner @janogarcia @germain-gg